-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: session replay respect feature flag variants #209
Conversation
@pauldambra would you mind taking a look? since I will need to implement this across SDKs |
@@ -49,6 +49,32 @@ class PostHogFeatureFlags { | |||
} | |||
} | |||
|
|||
private func isRecordingActive(_ featureFlags: [String: Any], _ sessionRecording: [String: Any]) -> Bool { | |||
var recordingActive = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is active since the sessionRecording
is a Map and not a boolean unless I find the linkedFlag
and can compare things
for item in samplesByTask where item.key.state == .completed { | ||
completedTasks[item.key] = item.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fixing a lint issue
@@ -120,15 +120,20 @@ class PostHogStorage { | |||
} | |||
|
|||
/** | |||
There are cases where applications using posthog-ios want to share analytics data between host app and an app extension, Widget or App Clip. If there's a defined `appGroupIdentifier` in configuration, we want to use a shared container for storing data so that extensions correcly identify a user (and batch process events) | |||
There are cases where applications using posthog-ios want to share analytics data between host app and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fixed a lint issue (line_length)
@ioannisj @pauldambra done, please double check after changes, I will do Android next (Monday) |
{ | ||
recordingActive = value == variant | ||
} | ||
// check for multi flag variant (any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we clean up these comments here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so since it's a valid result from the API and might help while debugging things
💡 Motivation and Context
JS PR PostHog/posthog-js#1040
💚 How did you test it?
📝 Checklist